-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: invocation spec #34
Conversation
0888838
to
256d777
Compare
278a5f1
to
545e70f
Compare
545e70f
to
90cdb89
Compare
Thanks for sharing @Gozala 🙌 PipelinesI like the visual pun of Expressivity
We looked at this in the original spec, but it made things much more difficult for the scheduler to understand cases like partial failure. For my own understating: do you have use case where you gain something from not putting success cases on an ok branch as a payload? Nonbinary Output
Can you say more about this? The UCAN Invocation spec as also supports structured output. SignaturesThat's... a lot of signatures. You came up with a possible idea of externalizing the signatures to avoid this last year, did that idea not work out? Is there intentionally no signature on the wrapper? Explicit DAGWhy the added layer of DAG? Does this have e.g. transactional semantics? We explored this idea in IPVM but ultimately decided against it. Our take (as well as most OCAP systems etc) was that the only semantically meaningful info is the dependencies between data. |
Unless I'm misunderstanding what you are saying here, I don't think would really change for the scheduler here. Because you just pipe specific branch, if it does not exists dependent task fails because it made wrong assumption. If you mean more broadly it's hard to tell if something is success or failure if you don't have ok or error, I do recognize it and perhaps language could be made more harsh so Result is mandatory as opposed to recommended. I'll elaborate on why I have not locked it to
By non-binary output I meant extensions of the Result variant like an example Perhaps all of that could be modeled with result instead, I'm open to reconsidering. However it did seem that making pipeline open to picking arbitrary branch was more flexible that locking it to ok / error logic.
I would argue there are less signatures, because here you are not required to delegate to the executor and then wrap that into invocation.
We could reduce signatures by signing a set indeed, that said I've decided it was premature optimization and decided to defer until we find a need for it.
Well there is no wrapper! It's just packet of invocations in CAR or JSON or whatever. If you want to batch them you pipe invocations into input of something like |
(Just between meetings, I'll give your response a closer look in a bit. Just a quick possible clarification)
Perhaps to clarify: there's no signature on this top level in your example. {
"aud": "did:web:ucan.run",
"iss": "did:key:z6Mkqa4oY9Z5Pf5tUcjLHLUsDjKwMC95HGXdE1j22jkbhz6r",
"with": "did:key:z6Mkqa4oY9Z5Pf5tUcjLHLUsDjKwMC95HGXdE1j22jkbhz6r",
"do": "data/identity", Without a signature, why are there |
Oh I see, that was a mistake it meant to have it just overlooked it. One in the spec does have it though Updated that example to fix this |
invocation.md
Outdated
|
||
with URI | ||
do Ability | ||
input In (implicit {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we're shortening key names like s
for the varsig, maybe this could be in
? fwiw I don't think that shortname is claimed in the jwt claim registry that we borrowed/were-inspired-by e.g. nbf
and iss
from https://www.iana.org/assignments/jwt/jwt.xhtml#claims
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also noticing that receipt has out
not output
so in
may fit better aesthetically, regardless of length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually wanted to use in
, but was hesitating for the same reason field named with
is really annoying (in JS) as you can't destructure or have variables with that name.
I don't mind renaming it to in
though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also considered use
because then you have "with Resource do Action, use Input"
invocation.md
Outdated
|
||
### 3.2.8 Nonce | ||
|
||
If present, the OPTIONAL `nnc` field MAY include a random nonce expressed in ASCII. This field can ensures that multiple invocations are unique. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's present but non-ascii, does that make it an invalid invocation? (before sig checks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what's the motivation here as it was inherited from @expede's spec. If I were doing it from scratch I'd just made nnc
bytes.
Let me provide more context here there are some napkin notes here , which I was hesitant to share which in turn motivated me to write up this PR. There are even more napkin notes, but let me try and summarize general idea here. We need to model system in which invoking a task may produce a result that is either
So what happens when we perform
flowchart LR
Idle("⛔️ not found") -- store/add -->
Active("🚦 active") --> Storage{Enough storage?}
Storage --> |Yes| S3{Is in S3 ?}
Storage --> |No| OverCapacity("⛔️ exceed capacity")
S3 --> |YES| Done("✅ stored")
S3 --> |No| WaitS3Put("🚦 receiving")
WaitS3Put -- store/put --> Done
WaitS3Put -- store/expire --> Expired("⛔️ expired")
Active -- store/receive --> S3
Active -- store/exceed --> OverCapacity
Edit: I've updated flow chart so it no longer misleads about cycles In other words our task can be in one of the following states
How this relates to this spec ?
We could and it in fact it may be better, to push I just don't know yet, in most cases I've found having third variant besides ok / error to be useful. E.g. generators in JS can fail, succeed or yield. Futures in Rust also seem to have ready / pending (where ready often wraps result). That is to say it felt like not locking it down seemed to provide more flexibility in the future, but I can also recognize how this ambiguity introduces complexity. In other words I don't mind making |
@Gozala thanks for writing up this Issue and pinging me on it 🎉 It was clarifying, though maybe not in the way that either of us was hoping for. I would like to chat though some of this on our scheduled Friday call if you still have interest! At minimum I'd like to see how I can still be helpful to you even if IPVM and W3S probably need different specs. I think my big take away from this is that we're trying to do very different things with this spec, which is where the tension is coming from. On the surface these look very similar, which is probably why it's so tempting to want to have both in one format. My comments below are under the assumption that we'd try to harmonize our specs and use cases. If we decide to just agree to disagree on this, then I'm also happy to give feedback for the DAG House use case without IPVM concerns. I like y'all and what you're doing, and want to help however I can, basically 💯 I'm ~99% sure that our proposals are equally expressive, but that they emphasize different things. To emphasize: I think we you can express everything from your current proposal in the UCAN Invocation, and vice versa, but they emphasize different things which makes each format awkward for the other.
|
I threw together a quick chart in case it's helpful. TL;DR your proposal with group signatures etc ticks a lot of these boxes that we couldn't agree on before — thank you! 🙏 Group signatures in particular seem like a broadly useful feature, and may be worth writing as an extension or technical report on Varsig to give others the pattern (sign a sorted IPLD array + take the CID and embed in other items).
The suggestion is that instead of us both including stuff that the other doesn't need or flagging a bunch of stuff as optional, we write a minimal spec that covers the stuff that we both need, and write further specs for the use-case specific stuff. We actually agree about most things! (I wasn't super granular about everything that we agree on about various parts of the fields in the Invocation etc but at this stage most of it overlaps I'm pretty sure!) |
That sounds good, but I think there are some nuances that do not align.
I see now what you meant, however I think it is a problem (for us anyway) we need receipt to point to the authorized invocation not just it's ID so they are fully verifiable without some additional context. If we leave out authorization they're not fully verifiable.
I assume that it's probably related to my point above, if not could you please elaborate on what you mean ? Is there a reason why you are hesitant to include For what it's worth in here #34 (comment) I've expanded
Another problem is promises, they should point to authorized invocation (unlike IPVM we can't expect every invocation to return same exact result across all users). Unless |
@expede I think only two ways we could align receipts
In terms of promises we also have couple of options
I am also open to exploring option no 2, if that seems preferable to you. If you see another way to align receipts and promises please let me know & we can explore them as well. |
invocation.md
Outdated
|
||
# Promise is a way to reference data from the output of the invocation | ||
type Promise struct { | ||
Await "<-" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor point because it's just the characters. I initially really liked the pun on <-
, but the overwhelming feedback internally at Fission is that it's difficult (or at least surprising and unfamialiar) to read, especially in combination with =
and ->
(which were in some version of this spec). Using words instead of symbols may be a good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not going to die on this hill, if <-
controversial I'm fine with whatever. Although I do find argument bit moot, it's something in binary format that is nothing anyone will ever manually type or read, in fact (other than pun) name was chosen so it would be very unlikely to collide with any other key someone might actually have on their structs.
invocation.md
Outdated
type Selector union { | ||
| Key String | ||
| Path [String] | ||
} kinded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We started down this path in the UCAN Invocation spec, but reverted it because of the complexity. Going down this path, we really quickly ran into edge cases that made us want more power, exploring using jsonpath, and so on. This kind of thing is why lenses exist 😉 We decided instead to just use a step in the workflow that's not baked into the core spec instead (i.e. we can use Wasm or a task type to do e.g. data shaping when consuming data from a previous Invocation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is some middle-ground between run WASM and just select ok / error, which is what I was trying to do here. JSONPath is pretty complex and I'm not surprised it did not pan out.
While I still kind of think that array of keys to traverse into is pragmatic and will get you far enough, I'm happy to let it go and maybe do it at other layer.
invocation.md
Outdated
type InvocationReference union { | ||
| Invocation<Any> | ||
| &Invocation<Any> | ||
} representation kinded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no problem with this, but can you expand on why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming you're asking here why allow inline invocations, no reason other than trying to keep some of the behavior from original invocation spec. I'm happy to get rid off it and use &Invocation
instead.
invocation.md
Outdated
nnc optional String | ||
nbf optional Int | ||
|
||
s Varsig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm contractually obligated to note that IPVM can't align on forcing a signature here, but this spec may not be totally up to date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After our talk I have created this PR to capture what I thought we agreed on https://github.com/web3-storage/specs/pull/35/files
I have not updated this PR as I was hoping we could align on remaining pieces before I go ahead and update everything else.
invocation.md
Outdated
"v": "0.1.0", | ||
"iss": "did:key:z6Mkqa4oY9Z5Pf5tUcjLHLUsDjKwMC95HGXdE1j22jkbhz6r", | ||
"aud": "did:web:ucan.run", | ||
"with": "javascript:(data) => data", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
invocation.md
Outdated
{ | ||
"iss": "did:key:z6Mkqa4oY9Z5Pf5tUcjLHLUsDjKwMC95HGXdE1j22jkbhz6r", | ||
"aud": "did:web:ucan.run", | ||
"with": "https://example.com/blog/posts", | ||
"do": "crud/create", | ||
"input": { | ||
"headers": { | ||
"content-type": "application/json" | ||
}, | ||
"payload": { | ||
"title": "How UCAN Tasks Changed My Life", | ||
"body": "This is the story of how one spec changed everything...", | ||
"topics": ["authz", "journal"], | ||
"draft": true | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is here purely for IPVM's benefit, I think we can get rid of this nested format completely. If we align on putting Auth
in the canonicalization for Promise
s, IIRC the reason for this that you had initially used this as a solve for probably just goes away.
At least for IPVM, we're a pretty hard no on this layout -- or at least it would take a lot of convincing. It makes it very difficult for us to reuse this computation elsewhere, complicates the parser, introduces multiple ways to call a secondary computation (inline and by CID), adds error cases that are not there otherwise, and has nested signatures.
(That doesn't mean that DAG House can't use this anyways because it's "just" a type of invocation, but making this a requirement in the spec is a sticking point for finding alignment IMO)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was just trying to show that you could have named tasks without building batches into spec, if links are fine for IPVM I'd gladly remove inlining.
invocation.md
Outdated
meta { String: Any } | ||
|
||
# Principal that issued this receipt | ||
iss Principal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you infer this from the underlying UCAN on the Invocation? What if they don't match?
invocation.md
Outdated
type Status<T, X, P> union { | ||
| T ("ok") # Success | ||
| X ("error") # Failure | ||
| P ("pending") # Pending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where would this be used? In an RPC call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a conversation about it other day where you've convinced me to just stick to ok / error. I have not updated PR to reflect this because my understanding was we wanted to align on group auth stuff first which is sketched in https://github.com/web3-storage/specs/pull/35/files
@Gozala Just looking at this stuff now!
Yeah, that's definitely nice! I think there's a lot of use cases for your design for signature envelopes with multiple payloads where you check for the CID of the target paylaod it's embedded in 💯 Once Varsig merges, we should consider at least writing a technical report suggesting it as a pattern.
I had written something else here before, but let me take a different approach: I understand that for DAG House the requirement is to have signatures per-invocation because you want to be able to toss them in a work-stealing queue. For IPVM, we would not need to submit individual invocations to the runtime at all, but rather send the Auth object itself (i.e. "a layer up"). Is the way you see IPVM using the embedded
Yeah agreed it's not a formal cycle, since it would require a hash cycle (which is "impractical"). Information theoretically, it does include redundant information, so the arrows form a loop when viewed one way (like my diagram above), but the other way to see this is something like this... flowchart TB
AuthedInvocation --> Invocation
AuthedInvocation --> Auth
Auth --> Invocation
...which still has redundant info, but doesn't appear as a cycle. |
TL;DR I think that (1) works for me 🎉 However, I'm not 100% sure that I follow, so let me try to rephrase: (1)The Scope object contains signatures and other metadata. In the simplest of cases, it is just an array that's signed over, but is open to being more expressive (e.g. globals, metadata, etc). We need a canonical representation of Invocations, so that we can address it by DID. If we include a pointer to the Scope in the Invocation's canonical CID, then we have away to address Invocations uniquely and with authentication, but without having to sign each of them independently or nest encryption. I'm pretty sure that this works for me 👍 It's essentially a minor change in where this information lives versus the current IPVM-focused proposal. I like that it gives us a constant CID across all use cases: Promises, Receipts, etc. (2)I think this one is a different expression of what's in the current Invocation proposal, where you end up pointing at the Scope and the specific Invocation inside it. If so, I think (1) is better 👍 (3)I think this is a variant of (2), because that data lives in the Promise. |
invocation.md
Outdated
|
||
# Proof that `iss` was authorized to by invocation `aud` to issue | ||
# receipts. Can be omitted if `job.aud === this.iss` | ||
prf [&UCAN] implicit ([]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually same for this, though the spec may not be entirely up to date: won't you be able to find this in the Invocation? Something like receipt.job.scope.prf
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So both iss
and prf
are to support the way our system is designed. Specifically client will sent invocation to did:web:web3.storage
but receipt will be issued by did:key:zKeyInJanuary
and prf
will include delegation chain did:web:web3.storage
to did:key:zKeyInJanuary
.
Also note that delegation will be pre-arranged as opposed to created on invocation, which is why we'd like to capture proof chain that actor that issued receipt was acting on behalf of executor.
prf
is already optional and I don't mind making iss
optional either in which case it would imply to be the aud
of the invocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
won't you be able to find this in the Invocation? Something like
receipt.job.scope.prf
?
You won't be able to find it in the invocation because client submitting invocation will not know what keys are in rotation or if we have a pool of workers that pick up task which one of would.
💯
I think you meant CID here, in which case exactly that
Exactly
Perfect! In that case I can update this PR to try to reflect this and address other pieces of feedback
Cool I did not like this option either, but this sketch left me under impression you didn't wanted to include scope in canonical invocation so I tried to be creative
Not exactly what I had in mind. In my head (1) just adds If (1) seems better I'm ok with doing that instead. |
@expede #34 (comment) this was basically option (3) with additional nuance that I wanted to make sure we have considered, specifically it defined invocation as a struct with single field and desired payload (I realize name proved controversial, so lets say it's some unique descriptive name instead of
It is equivalent of multi(thing) in IPFS parley, it's self describing via key prefix. It means in a different context I could define another union where invocation is just one of the variants. Just like other multihings it is possible to infer what is it from it's bytes as opposed to from the context. I think overhead is negligible while potential is high, which is why I think it is worth considering regardless if we choose (1) or (3). |
Updates schema and example as per sketch https://www.tldraw.com/r/1673637514425 with @expede
@expede I have update the spec, could you please take another look and let me know what you think ? Also could you please let me know if you had a chance to evaluate idea of handling batching using special capability like |
Closing this in favor of ucan-wg/invocation#10 |
📜 Preview
± Diff with ucan invocation spec
This is the fork of ucan invocation spec authored by @expede that alters it as follows:
data/identity
could be used to do basically what batches accomplish in original specRefers to whole task result (regardless of success / failure)
Specific result branch (success / failure / ...)
Or a specific value inside a specific branch
If wrong branch is pipelined into invocation than invocation MUST fail
Result
as long as it's kinded union branching will workResult
or extension with added variants when binary ok / error is not enoughResult
requirement). That is to allow extensions that model long running tasks that may transition through multiple states. It is out of scope here, but point is it could be implemented without violating spec and promise pipelining would also work.aud
be an Executor and allow them to beiss
, that way we don't have to first create a delegation and then equivalent invocation. It does state however that unlessaud
is Executor it will not be able to redelegate such proofs.I hope this draft provides a better way to articulate issues we had with original spec and provides side by side comparison. @expede I would love your feedback